Skip to content

Allow running reduce_parent operations on stack allocated parents#7751

Open
robert3005 wants to merge 18 commits into
developfrom
refactor/parent-ref-stack-allocated
Open

Allow running reduce_parent operations on stack allocated parents#7751
robert3005 wants to merge 18 commits into
developfrom
refactor/parent-ref-stack-allocated

Conversation

@robert3005
Copy link
Copy Markdown
Contributor

Add indirection logic to support stack allocated array parents in reduce rules.

This lets us avoid heap allocation for commonly used operations that are often
immediately optimised away

@robert3005 robert3005 force-pushed the refactor/parent-ref-stack-allocated branch from 3751910 to 08c9f4a Compare May 1, 2026 14:51
Comment thread vortex-array/src/array/erased.rs Outdated
Comment thread vortex-array/src/matcher.rs Outdated
Comment thread vortex-array/src/array/parent.rs
@codspeed-hq
Copy link
Copy Markdown

codspeed-hq Bot commented May 1, 2026

Merging this PR will improve performance by 18.04%

⚠️ Unknown Walltime execution environment detected

Using the Walltime instrument on standard Hosted Runners will lead to inconsistent data.

For the most accurate results, we recommend using CodSpeed Macro Runners: bare-metal machines fine-tuned for performance measurement consistency.

⚡ 2 improved benchmarks
✅ 1273 untouched benchmarks

Performance Changes

Mode Benchmark BASE HEAD Efficiency
Simulation slice_dict_tight_loop[10000] 351.1 µs 300.2 µs +16.96%
Simulation slice_primitive_tight_loop[10000] 178.9 µs 150.2 µs +19.13%

Tip

Curious why this is faster? Comment @codspeedbot explain why this is faster on this PR, or directly use the CodSpeed MCP with your agent.


Comparing refactor/parent-ref-stack-allocated (1e66f4a) with develop (23ebab1)

Open in CodSpeed

Comment thread vortex-array/src/array/typed.rs Outdated
Comment thread encodings/fastlanes/src/bitpacking/vtable/operations.rs
@joseph-isaacs joseph-isaacs added the action/benchmark Trigger full benchmarks to run on this PR label May 1, 2026
@robert3005 robert3005 added changelog/break A breaking API change changelog/performance A performance improvement and removed action/benchmark Trigger full benchmarks to run on this PR changelog/performance A performance improvement labels May 1, 2026
Comment thread vortex-array/src/array/typed.rs Outdated
@robert3005 robert3005 force-pushed the refactor/parent-ref-stack-allocated branch 2 times, most recently from 0b38af3 to 95b0670 Compare May 10, 2026 19:28
@joseph-isaacs
Copy link
Copy Markdown
Contributor

Some of those benchmarks are doing extra real work

@robert3005
Copy link
Copy Markdown
Contributor Author

robert3005 commented May 11, 2026

This is not ready yet, there’s still something wrong with matchers

@robert3005 robert3005 force-pushed the refactor/parent-ref-stack-allocated branch from 95b0670 to 04c7c59 Compare May 11, 2026 09:44
@robert3005 robert3005 marked this pull request as draft May 11, 2026 09:54
@robert3005 robert3005 force-pushed the refactor/parent-ref-stack-allocated branch 2 times, most recently from 7401ce4 to 023b630 Compare May 18, 2026 19:24
@robert3005 robert3005 marked this pull request as ready for review May 19, 2026 22:51
@robert3005 robert3005 requested a review from gatesn May 22, 2026 11:25
@robert3005 robert3005 force-pushed the refactor/parent-ref-stack-allocated branch from 380cc8b to feac477 Compare May 28, 2026 17:19
@robert3005 robert3005 requested a review from joseph-isaacs May 28, 2026 23:12
robert3005 and others added 5 commits May 30, 2026 17:37
`ArrayRef::slice/filter/take` previously allocated a wrapper array
(`SliceArray`, `FilterArray`, `DictArray`) and called `.optimize()` on
the resulting `ArrayRef`, relying on a `reduce_parent` rule to throw the
wrapper away. The wrapper allocation was always paid even when reduction
ran in one shot.

This change establishes the API surface for moving the wrapper onto the
stack:

- `ParentRef<'a>`: a parent representation that optionally borrows an
  `&ArrayRef` and otherwise carries the encoding-specific data, dtype,
  length, slots, and encoding id directly. `into_array_ref(self)` clones
  the underlying `Arc` when the parent is heap-backed.
- `ParentView<'a, V>`: a typed view of a parent that derefs to
  `V::ArrayData` without holding an `&ArrayRef`. Used by the upcoming
  matcher path that accepts stack-allocated parents.
- `DynArray::data_any` exposes the encoding-specific data so a matcher
  can downcast to `V::ArrayData` from a `ParentRef` regardless of
  whether the parent is heap- or stack-backed.
- `ArrayParts<V>::optimize`, `optimize_ctx(session)`, and `into_array`,
  plus `Optimized<V>` with its own `into_array`. Callers chain
  `parts.optimize()?.into_array()` so reduction is an explicit,
  orthogonal step from materialization.
- `ArrayRef::slice / filter / take` now build an `ArrayParts<V>` and
  drive it through the chain.

The internals of `ArrayParts::optimize` still materialize before
running the existing `reduce_parent` chain, so this PR does not yet
remove the `Arc<ArrayInner<V>>` allocation. Wiring `ParentRef` through
`DynArray::reduce_parent`, `VTable::reduce_parent`, `ParentRuleSet`,
`DynArrayParentReduceRule`, `ReduceParentFn`, the `Matcher` API, and
the per-encoding rule bodies is the follow-up that delivers the
allocation savings.

- `cargo build --workspace`
- `cargo nextest run -p vortex-array -p vortex-fastlanes -p vortex-fsst -p vortex-alp -p vortex-runend -p vortex-zigzag` (all 3078 pass; the 21 skipped are timezone-dependent and unrelated to this change)
- `cargo clippy -p vortex-array --all-targets --all-features`
- `cargo +nightly fmt --all -- --check`
- `./scripts/public-api.sh`

Signed-off-by: Robert Kruszewski <github@robertk.io>
Signed-off-by: Robert Kruszewski <github@robertk.io>
Signed-off-by: Robert Kruszewski <github@robertk.io>
Signed-off-by: Robert Kruszewski <github@robertk.io>
Signed-off-by: Robert Kruszewski <github@robertk.io>
Signed-off-by: Robert Kruszewski <github@robertk.io>
Signed-off-by: Robert Kruszewski <github@robertk.io>
Signed-off-by: Robert Kruszewski <github@robertk.io>
Signed-off-by: Robert Kruszewski <github@robertk.io>
Signed-off-by: Robert Kruszewski <github@robertk.io>
Signed-off-by: Robert Kruszewski <github@robertk.io>
Signed-off-by: Robert Kruszewski <github@robertk.io>
Signed-off-by: Robert Kruszewski <github@robertk.io>
Signed-off-by: Robert Kruszewski <github@robertk.io>
@robert3005 robert3005 force-pushed the refactor/parent-ref-stack-allocated branch from 1da7104 to 94594c6 Compare May 30, 2026 16:37
Signed-off-by: Robert Kruszewski <github@robertk.io>
Signed-off-by: Robert Kruszewski <github@robertk.io>
Signed-off-by: Robert Kruszewski <github@robertk.io>
Signed-off-by: Robert Kruszewski <github@robertk.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

changelog/break A breaking API change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants